Skip to content

Conversation

@AustinChangLinksys
Copy link
Collaborator

@AustinChangLinksys AustinChangLinksys commented Dec 30, 2025

Summary

This PR introduces the Router AI Assistant feature and performs dependency cleanup for the PrivacyGUI application.

New Features

🤖 Router AI Assistant

  • A2UI Rendering: AI-powered UI generation using the A2UI protocol
  • Context-aware responses: System prompt includes real-time router state
  • Conversation History: Full message history with inline A2UI component display
  • Loading indicator: Visual feedback during AI processing

💬 FAQ Agent

  • AWS Bedrock Integration: LLM-powered FAQ analysis
  • Direct Search Fallback: Works without AWS credentials
  • Fixed URL: Corrected article links to /kb/article/ format

Code Changes

New Modules

  • lib/ai/ - AI abstraction layer, orchestrator, prompts, and component registry
  • lib/page/ai_assistant/ - Router Assistant view
  • lib/page/support/widgets/faq_agent_fab.dart - FAQ Agent FAB widget

Improvements

  • Language: AI responds in user's language (not forced Chinese)
  • Strings: All AI module strings translated to English
  • UI Kit: Upgraded to GitHub dependency v2.10.1

Dependency Cleanup

Removed unused packages:

  • flutter_fancy_tree_view (replaced by UI Kit TopologyTreeView)
  • numberpicker
  • graphview
  • phone_numbers_parser

Documentation

  • doc/ai_assistant/router_ai_assistant.md - Workflow documentation
  • doc/ai_assistant/router_assistant_architecture.md - Architecture with class diagrams
  • doc/ai_assistant/FAQ_AGENT.md - FAQ Agent documentation

…ory and cache support

- Add DemoRouterRepository to intercept JNAP requests and serve cached data
- Add JnapMockRegistry and DemoCacheDataLoader
- Add demo_cache_data.json asset
- Update demo_overrides.dart:
  - Override routerRepositoryProvider
  - Auto-start polling
  - Mock geolocationProvider with fake Irvine, CA data to avoid cloud errors
- Configure _DemoServiceHelper for correct feature support
- Add usp_client_core package with:
  - PollingManager (non-singleton, injectable)
  - ResourceWatcher with push/pull hybrid strategy
  - WatchStrategy enum for monitoring modes
  - UspCapabilityService for TR-181 schema discovery

- Add USP capabilities system in lib/core/usp/:
  - CapabilityRepository interface and implementations
  - Device feature enum for capability checking
  - Riverpod providers with override support
  - UspWifiRepository for WiFi monitoring

- Add main_usp_demo.dart entry point for USP mode
- Update main_usp_demo dependencies to use capability injection
- Design ensures main.dart and main_demo.dart are unaffected
@qodo-code-review
Copy link

qodo-code-review bot commented Dec 30, 2025

PR Compliance Guide 🔍

(Compliance updated until commit f5c1675)

Below is a summary of compliance checks for this PR:

Security Compliance
Sensitive data logging

Description: Debug logging prints full raw TR-181 key/value data and mapped JNAP outputs (including
potentially sensitive fields like WiFi passphrases from
Device.WiFi.AccessPoint.*.Security.KeyPassphrase, MAC addresses, serial numbers, and IPs),
which can leak secrets/PII into application logs if enabled in non-development
environments.
jnap_tr181_mapper.dart [144-1166]

Referred Code
logger.d('🔄 Mapping USP response for $actionName (base: $baseName)');
logger.d('   Raw USP values: ${response.results.length} items');

// Print raw TR-181 data received
final buffer = StringBuffer();
buffer.writeln('   📦 Raw TR-181 Data:');
for (final entry in response.results.entries) {
  buffer.writeln('      ${entry.key.fullPath} = ${entry.value.value}');
}
logger.d(buffer.toString());

Map<String, dynamic> result;
switch (baseName) {
  case 'GetDeviceInfo':
    result = _mapDeviceInfo(response);
    break;
  case 'GetRadioInfo':
    result = _mapRadioInfo(response);
    break;
  case 'GetDevices':
    result = _mapDevices(response);


 ... (clipped 1002 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: 🏷️
Sensitive data logging: The new mapper logs raw TR-181 values and mapped outputs that can include sensitive data
such as Wi-Fi passphrases, MAC addresses, and IPs, violating secure logging requirements.

Referred Code
logger.d('🔄 Mapping USP response for $actionName (base: $baseName)');
logger.d('   Raw USP values: ${response.results.length} items');

// Print raw TR-181 data received
final buffer = StringBuffer();
buffer.writeln('   📦 Raw TR-181 Data:');
for (final entry in response.results.entries) {
  buffer.writeln('      ${entry.key.fullPath} = ${entry.value.value}');
}
logger.d(buffer.toString());

Map<String, dynamic> result;
switch (baseName) {
  case 'GetDeviceInfo':
    result = _mapDeviceInfo(response);
    break;
  case 'GetRadioInfo':
    result = _mapRadioInfo(response);
    break;
  case 'GetDevices':
    result = _mapDevices(response);


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: 🏷️
Exposes secrets in output: The new mapping logic reads and returns sensitive fields (e.g.,
Device.WiFi.AccessPoint.*.Security.KeyPassphrase) into response maps and also logs them,
which can expose secrets without sanitization or access control.

Referred Code
  'security': values['$apPrefix.Security.ModeEnabled']?.toString() ??
      'WPA2-Personal',
  'wpaPersonalSettings': {
    'passphrase':
        values['$apPrefix.Security.KeyPassphrase']?.toString() ?? '',
  },
},

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: 🏷️
Non-audit debug logs: The new code adds extensive debug logging but does not implement structured audit trails
with user ID, timestamp, action, and outcome for critical actions, requiring verification
of whether critical actions exist in this flow and are audited elsewhere.

Referred Code
logger.d('🔄 Mapping USP response for $actionName (base: $baseName)');
logger.d('   Raw USP values: ${response.results.length} items');

// Print raw TR-181 data received
final buffer = StringBuffer();
buffer.writeln('   📦 Raw TR-181 Data:');
for (final entry in response.results.entries) {
  buffer.writeln('      ${entry.key.fullPath} = ${entry.value.value}');
}
logger.d(buffer.toString());

Map<String, dynamic> result;
switch (baseName) {
  case 'GetDeviceInfo':
    result = _mapDeviceInfo(response);
    break;
  case 'GetRadioInfo':
    result = _mapRadioInfo(response);
    break;
  case 'GetDevices':
    result = _mapDevices(response);


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: 🏷️
Silent parsing fallback: The new code uses broad fallback defaults and swallows parsing errors (e.g., subnet
parsing) without logging context, which may hinder production debugging and needs
confirmation of expected behavior and monitoring strategy.

Referred Code
int _subnetMaskToPrefix(String subnetMask) {
  if (subnetMask.isEmpty) return 24;
  try {
    final parts = subnetMask.split('.');
    if (parts.length != 4) return 24;
    int prefix = 0;
    for (final part in parts) {
      int octet = int.parse(part);
      while (octet > 0) {
        prefix += octet & 1;
        octet >>= 1;
      }
    }
    return prefix;
  } catch (_) {
    return 24;
  }
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: 🏷️
Potentially verbose errors: The new code throws UspException messages containing internal type and protocol details
which may be surfaced to end-users depending on upstream handling, requiring verification
that these messages are not displayed user-facing.

Referred Code
    throw UspException(7004, "Unsupported DTO type: ${dto.runtimeType}");
  }

  return pb.Msg()
    ..header = header
    ..body = body;
}

/// Converts a Protobuf [pb.Msg] to a [UspMessage] DTO.
UspMessage fromProto(pb.Msg protoMsg) {
  final body = protoMsg.body;

  switch (body.whichMsgBody()) {
    case pb.Body_MsgBody.request:
      return _parseRequest(body.request);
    case pb.Body_MsgBody.response:
      return _parseResponse(body.response);
    case pb.Body_MsgBody.error:
      return UspErrorResponse(
        UspException(body.error.errCode, body.error.errMsg),
      );



 ... (clipped 3 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit fcbe8ef
Security Compliance
Sensitive info exposure

Description: Debug logging prints raw TR-181 key/value data (including potentially sensitive fields
like WiFi passphrases, MAC addresses, serial numbers, IPs) via logger.d and could leak
secrets to logs or crash reports if enabled outside a strictly controlled dev environment.

jnap_tr181_mapper.dart [144-153]

Referred Code
logger.d('🔄 Mapping USP response for $actionName (base: $baseName)');
logger.d('   Raw USP values: ${response.results.length} items');

// Print raw TR-181 data received
final buffer = StringBuffer();
buffer.writeln('   📦 Raw TR-181 Data:');
for (final entry in response.results.entries) {
  buffer.writeln('      ${entry.key.fullPath} = ${entry.value.value}');
}
logger.d(buffer.toString());
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: 🏷️
Incorrect default logic: The DHCP enable flag computation is effectively hardcoded to true, preventing correct
handling of the false/disabled edge case.

Referred Code
final isDhcpEnabled = values['Device.DHCPv4.Server.Enable'] == 'true'
    ? true
    : true; // Default true if missing
final hostName = values['Device.DeviceInfo.HostName'] ?? 'Linksys01234';

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: 🏷️
Sensitive data logged: Debug logging prints raw TR-181 key/value pairs and the full mapped output, which can
include secrets/PII such as WiFi passphrases, MAC addresses, and IP addresses.

Referred Code
logger.d('🔄 Mapping USP response for $actionName (base: $baseName)');
logger.d('   Raw USP values: ${response.results.length} items');

// Print raw TR-181 data received
final buffer = StringBuffer();
buffer.writeln('   📦 Raw TR-181 Data:');
for (final entry in response.results.entries) {
  buffer.writeln('      ${entry.key.fullPath} = ${entry.value.value}');
}
logger.d(buffer.toString());

Map<String, dynamic> result;
switch (baseName) {
  case 'GetDeviceInfo':
    result = _mapDeviceInfo(response);
    break;
  case 'GetRadioInfo':
    result = _mapRadioInfo(response);
    break;
  case 'GetDevices':
    result = _mapDevices(response);


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: 🏷️
Error details exposed: The converter serializes dto.exception.message into the protobuf pb.Error.errMsg, which
may expose internal details to external consumers depending on how errors are surfaced.

Referred Code
    case UspErrorResponse():
      final err = pb.Error()
        ..errCode = dto.exception.errorCode
        ..errMsg = dto.exception.message; // 假設你的 exception 有 message 欄位
      body.error = err;
  }
} else {
  throw UspException(7004, "Unsupported DTO type: ${dto.runtimeType}");
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: 🏷️
Secret handling unclear: The mapper propagates and (via current debug logging) may handle sensitive fields like
Device.WiFi.AccessPoint.*.Security.KeyPassphrase without clear redaction/guarding, and it
is not visible in the diff whether these values are protected in release builds.

Referred Code
'security': values['$apPrefix.Security.ModeEnabled']?.toString() ??
    'WPA2-Personal',
'wpaPersonalSettings': {
  'passphrase':
      values['$apPrefix.Security.KeyPassphrase']?.toString() ?? '',
},

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-code-review
Copy link

qodo-code-review bot commented Dec 30, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Replace hardcoded mapper with data-driven engine

Refactor the JnapTr181Mapper from a hardcoded implementation using maps and
switch statements to a more scalable, data-driven engine. This new engine would
use an external configuration file (like JSON) to define the translation logic.

Examples:

lib/core/usp/jnap_tr181_mapper.dart [15-209]
class JnapTr181Mapper {
  /// Maps JNAP Action names to their corresponding TR-181 paths.
  ///
  /// Note: Only include actions here if toJnapResponse has a proper handler.
  /// Actions without handlers should use fallback responses instead.
  ///
  /// GetDeviceInfo is NOT included here intentionally - the app needs the
  /// full services list (150+ items) from demo_cache_data.json for
  /// buildBetterActions() to work properly.
  ///

 ... (clipped 185 lines)

Solution Walkthrough:

Before:

class JnapTr181Mapper {
  static const _actionPathMappings = <String, List<String>>{
    'GetDeviceInfo': ['Device.DeviceInfo.'],
    'GetRadioInfo': ['Device.WiFi.Radio.', /* ... */],
    // ... many more hardcoded mappings
  };

  Map<String, dynamic> toJnapResponse(JNAPAction action, UspGetResponse response) {
    final baseName = _stripVersionSuffix(action.actionValue.split('/').last);
    switch (baseName) {
      case 'GetDeviceInfo':
        return _mapDeviceInfo(response);
      case 'GetRadioInfo':
        return _mapRadioInfo(response);
      // ... many more cases with hardcoded logic
      default:
        return _mapGeneric(response);
    }
  }

  Map<String, dynamic> _mapDeviceInfo(UspGetResponse response) {
    final values = _flattenResults(response);
    return {
      'manufacturer': values['Device.DeviceInfo.Manufacturer'] ?? 'Unknown',
      'modelNumber': values['Device.DeviceInfo.ModelName'] ?? 'Unknown',
      // ... more hardcoded field mappings
    };
  }
}

After:

// In a separate config file (e.g., jnap_tr181_mapping.json)
/*
{
  "GetDeviceInfo": {
    "tr181_paths": ["Device.DeviceInfo."],
    "response_mapping": {
      "manufacturer": { "path": "Device.DeviceInfo.Manufacturer", "default": "Unknown" },
      "modelNumber": { "path": "Device.DeviceInfo.ModelName", "default": "Unknown" },
      "services": { "value": ["http://.../Core", "..."] }
    }
  },
  // ... other action mappings
}
*/

class JnapTr181Mapper {
  final Map<String, dynamic> _mappingConfig;

  JnapTr181Mapper(String configJson) : _mappingConfig = json.decode(configJson);

  Map<String, dynamic> toJnapResponse(JNAPAction action, UspGetResponse response) {
    final actionName = ...;
    final config = _mappingConfig[actionName];
    if (config == null) return {};

    // Generic engine parses config and applies transformations
    return _mappingEngine.transform(response, config['response_mapping']);
  }
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant architectural weakness in the new JnapTr181Mapper, which uses hardcoded logic, making it brittle and hard to maintain.

High
Possible issue
Fix incorrect DHCP status evaluation
Suggestion Impact:Replaced the faulty ternary that always evaluated to true with a null-safe string comparison that defaults to 'true' when the response value is missing, correctly determining DHCP enablement.

code diff:

-    final isDhcpEnabled = values['Device.DHCPv4.Server.Enable'] == 'true'
-        ? true
-        : true; // Default true if missing
-    final hostName = values['Device.DeviceInfo.HostName'] ?? 'Linksys01234';
+    final isDhcpEnabled =
+        (values['Device.DHCPv4.Server.Enable']?.toString() ?? 'true') == 'true';

The current logic always sets isDhcpEnabled to true. Correct the expression to
properly check the value from the USP response, defaulting to true if it's
missing.

lib/core/usp/jnap_tr181_mapper.dart [817-819]

-final isDhcpEnabled = values['Device.DHCPv4.Server.Enable'] == 'true'
-    ? true
-    : true; // Default true if missing
+final isDhcpEnabled =
+    (values['Device.DHCPv4.Server.Enable']?.toString() ?? 'true') == 'true';

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logical flaw where isDhcpEnabled is always true due to a faulty ternary expression, and the proposed fix is concise and correct.

Medium
Avoid empty-path leading dots

In _parseGetResp, add a check for an empty objPath before concatenating it with
paramName to prevent creating invalid paths that start with a dot.

packages/usp_protocol_common/lib/src/converter/usp_protobuf_converter.dart [692-703]

 for (final resolved in reqRes.resolvedPathResults) {
   final objPath = resolved.resolvedPath;
   resolved.resultParams.forEach((paramName, val) {
-    final fullParamPath = UspPath.parse(
-      "$objPath.$paramName",
-    );
+    final pathString = objPath.isEmpty
+        ? paramName
+        : "$objPath.$paramName";
+    final fullParamPath = UspPath.parse(pathString);
     results[fullParamPath] = UspValue(
       val,
       UspValueType.string,
     );
   });
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a potential issue where an empty objPath could result in an invalid path string like ".paramName", and provides a simple conditional check to prevent it.

Low
General
Correct version suffix stripping

Update the regex in _stripVersionSuffix to use \d+ instead of [2-9]+ to
correctly remove all trailing numeric version suffixes, including those with "1"
or multiple digits.

lib/core/usp/jnap_tr181_mapper.dart [121-124]

 String _stripVersionSuffix(String actionName) {
-  final regex = RegExp(r'[2-9]+\$');
+  final regex = RegExp(r'\d+\$');
   return actionName.replaceFirst(regex, '');
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the regex [2-9]+$ fails to strip multi-digit version suffixes like "10" and provides a more robust regex \d+$ to handle all trailing digits.

Low
  • Update

Update usages in login_cloud_view.dart and login_local_view.dart
to use the new hintText parameter.
- Add Router AI Assistant with A2UI rendering
- Add FAQ Agent FAB with AWS Bedrock integration
- Fix FAQ URL path to /kb/article/
- Update system prompt: respond in user's language
- Translate Chinese strings to English
- Switch UI Kit to GitHub dependency (v2.10.1)
- Remove unused dependencies: flutter_fancy_tree_view, numberpicker, graphview, phone_numbers_parser
- Add AI assistant documentation
@AustinChangLinksys AustinChangLinksys changed the title feat: update ui_kit to v2.8.0 and enable topology animation in demo feat(ai): Add Router AI Assistant and cleanup dependencies Jan 5, 2026
Fix invalid_override errors by changing sideEffectOverrides to pollConfig:
- UspMapperRepository.send() and transaction()
- DemoRouterRepository.send() and transaction()
- pubspec.yaml: bundle env.template instead of assets/.env
- main.dart: load env.template
- main_demo.dart: load env.template

.env is gitignored, so CI cannot find it. env.template is committed.
- ci.yml: Add 'cp env.template assets/.env' before flutter pub get
- pubspec.yaml: Keep assets/.env in assets list

This ensures CI can build with the required .env asset file.
@AustinChangLinksys
Copy link
Collaborator Author

AustinChangLinksys commented Jan 5, 2026

ecurity Compliance Review - Justification Notes

  1. ✅ DHCP Enable Logic (Fixed)
    Issue: Hardcoded true in ternary expression.
    Resolution: Fixed in commit. Changed from ? true : true to == 'true' for correct boolean evaluation.

  2. ✅ Secure Logging Practices (False Positive)
    Issue: Debug logs print raw TR-181 data including potentially sensitive information.
    Justification: The application uses the logger package, which is configured to disable all logging output in release builds. Sensitive data is only visible during development/debugging and never exposed in production binaries.

  3. ✅ Secure Error Handling (Low Risk - Accepted)
    Issue: Error messages serialized to protobuf may expose internal details.
    Justification: The protobuf messages are used exclusively for local inter-process communication between the Flutter application and the router. They are never transmitted to external servers or exposed to public networks. The error messages are necessary for proper debugging and user feedback within the local application context.

  4. ✅ Secret Handling - WiFi Passphrase (False Positive)
    Issue: WiFi passphrases may be logged without redaction.
    Justification: Same as Update project structures #2 - the logger package is configured to disable logging in release builds. Additionally, this application is a router management tool where handling WiFi credentials is part of the core functionality. The data flow is local (app ↔ router) and passphrases are never transmitted to external services.

Use null-coalescing to default to 'true' when Device.DHCPv4.Server.Enable
is missing, matching expected router behavior where DHCP is typically enabled.
@PeterJhongLinksys PeterJhongLinksys merged commit fa46798 into dev-2.0.0 Jan 5, 2026
2 checks passed
@PeterJhongLinksys PeterJhongLinksys deleted the feature/demo-application-fix-topbar branch January 5, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants